Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tree-explorer): add buttons to ask Copilot and create playgrounds from tree view VSCODE-651 #890

Merged
merged 39 commits into from
Dec 6, 2024

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Nov 28, 2024

Includes everything in VSCODE-651 other than telemetry as I ended up doing some minor refactoring in it for in #891

@gagik gagik changed the base branch from main to gagik/add-icons November 28, 2024 12:47
Base automatically changed from gagik/add-icons to main November 28, 2024 13:16
@gagik gagik changed the title WIP feat: playground buttons feat(tree-explorer): Add buttons to ask Copilot and create playgrounds from tree view VSCODE-651 Nov 29, 2024
@gagik gagik marked this pull request as ready for review November 29, 2024 12:27
@gagik gagik force-pushed the gagik/add-playground-buttons branch from b32976e to 82cff2a Compare November 29, 2024 12:31
@gagik gagik changed the title feat(tree-explorer): Add buttons to ask Copilot and create playgrounds from tree view VSCODE-651 feat(tree-explorer): add buttons to ask Copilot and create playgrounds from tree view VSCODE-651 Nov 29, 2024
@@ -0,0 +1,10 @@
/** Wraps a template function and escapes given string arguments. */
export function createTemplate<T extends (...args: string[]) => string>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're going for string escaping, I thought it might be nice to switch to something more type-safe and use functions + string templates, would help keep track of variables used better.

I could convert other templates at a later time also, but this could be a nice starting point, unless there's benefits to replacing that I might be overseeing.

In case we end up with content that needs to not be escaped somehow, we could expand this function to support more complex cases but for now I just wanted to create a wrapper.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to use handlebars or a similar templating engine rather than roll something ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that might be nice though not sure how complex our templates can get; in any case I'd maybe personally prefer an engine written with TypeScript in mind like eta

With stuff like replacing characters we'd probably end up handling it ourselves or use a plugin with these template engines. Honestly to me having a function build a string seems like a nicer experience mainly because of arguments being exposed in the IDE though perhaps a template engine would be more scalable for the future.

cc: @alenakhineika

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Gaurab we have discussed a revamp of how we do templates with things like adding shortcut information to them, it might be a better moment to discuss introducing i.e. a templating engine, I created VSCODE-661 to keep track of that.

I could go ahead with this setup for now just to have it in for release and we can change it to whatever we decide more globally later.

src/mdbExtensionController.ts Outdated Show resolved Hide resolved
src/mdbExtensionController.ts Outdated Show resolved Hide resolved
src/participant/participant.ts Outdated Show resolved Hide resolved
src/participant/participant.ts Outdated Show resolved Hide resolved
src/templates/templateHelpers.ts Show resolved Hide resolved
@gagik gagik merged commit fabbc8c into main Dec 6, 2024
6 checks passed
@gagik gagik deleted the gagik/add-playground-buttons branch December 6, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants